fix(cases): bound path probes for linked workspaces and session creation, so an unreachable mount cannot freeze the server - #516

DRAFT_COMMENT

Hi @Ark0N, it looks like your comment here came through as the placeholder text "DRAFT_COMMENT", so I think the actual review didn't post. Happy to pick it up whenever you get a chance to re-send it.

Thanks for this, @aakhter, and sorry for the "DRAFT_COMMENT" placeholder that went out here on 10-01. That was a glitch in my review tooling, and this is the review that should have been posted.

This PR stops a linked case on an unreachable network mount from freezing the server. boundedPathExists() stats asynchronously, gives up after 1.5 s, shares one in-flight probe per path, remembers a stalled path until its stat settles, and stops starting new probes while two stalled ones hold threadpool workers. The stall mechanics are carefully done: no unhandled rejections, timers cleared, and the route test proves GET /api/cases keeps answering (it fails on master). That part is worth keeping as is. Typecheck, lint, format, the touched-area tests and CI all pass here, and the branch still merges cleanly onto 1.34.0.

What needs another round is how callers read a non-answer. Today "did not answer in 1.5 s" and "refused by the cap" both come back as false, i.e. "absent", and in two places that causes something worse than the freeze. Both were confirmed with throwaway tests on your own route harness.

Must fix

1. A slow or stalled case now 404s, and the Run button answers a 404 by creating a stray local case (src/web/routes/case-routes.ts:1641, with src/web/public/session-ui.js:1869 and :2077). GET /api/cases/:name returns NOT_FOUND whenever boundedPathExists(casePath) is false. runClaude() and runShell() call that endpoint first and, when the answer has no data.path, POST /api/cases to create it. For a linked case that POST succeeds: it scaffolds ~/codeman-cases/<name> with a template CLAUDE.md and hooks, and the session starts there instead of in the user's project. Once the mount recovers, the stray directory shadows the linked case in GET /api/cases (local entries are listed first, and the linked loop skips names it has already seen), while GET /api/cases/:name resolves the linked path, so the two endpoints disagree from then on. Under the cap from item 2, a healthy local case 404s too, the follow-up POST fails with ALREADY_EXISTS, and Run shows "Case already exists".

   Please make the probe distinguish "absent" from "unknown": a tri-state ('present' | 'absent' | 'unknown'), or a boolean plus a separate timedOut. Keep NOT_FOUND for a definite absence only. For "unknown", either return the registered path (the linked registry already knows it) or return an error that is not NOT_FOUND, and make runClaude/runShell create a case only on errorCode === 'NOT_FOUND'. Please add a route test for the timeout path of GET /api/cases/:name.

2. The stalled-probe cap is process-wide, so two stalled paths make every path read as absent (src/utils/bounded-path-probe.ts:58). Protecting the threadpool is the right concern, but the refusal also answers false for paths that have nothing to do with the dead mount. On a hard mount the stalled stats don't settle until the mount comes back, which can take hours. One dead NAS reaches two stalled paths easily: two linked cases on it, or one linked case plus its CLAUDE.md. While that lasts:

   • applyWorkspaceHooks() (src/hooks-config.ts:841) returns early for every workspace, so every new claude session, cron run and boot-recovered session runs without Codeman's hooks. That means no permission-prompt alert, no Approvals Inbox item, no push, and no stop/idle_prompt for respawn or the wait endpoints, and nothing is logged. A throwaway test confirmed it: a healthy temp workspace got no settings.local.json while two unrelated paths were stalled, and got one after they were released.
   • every case's hasClaudeMd reads false, and GET /api/cases/:name 404s for every case (feeding item 1).
   • resolveStatusLineCliCommand() (:1120) misses the user's own statusLine, so the plan-usage exporter is injected over it, against that function's own "never override a real one".
   • the clone flow's repo-settings warning disappears (item 3), and GET /api/cases/:name/fix-plan reports no plan.

   Your unit test pins this on purpose (/healthy/three reads false while two other paths are stalled), so the gap is the effect on the callers rather than the helper itself. Either of these works, your choice: scope the refusal to paths near a stalled one (same linked-case root or same leading mount segments) and probe everything else normally, or return "unknown" when refusing and let each caller decide (hook installation should go ahead for a workspace that is not itself stalled, and a listing can mark the case unreachable). Either way, please console.warn once when a path first stalls and once when the cap engages, so "my case vanished" and "hooks stopped firing" leave a trace. And please add a test that a healthy path still probes true while two unrelated paths are stalled.

Should fix

3. Keep the clone flow's security warning off the bounded probe (src/web/routes/case-routes.ts:166-171, repoShipsClaudeSettings). It runs on a directory that was just cloned into the local case space, right after a synchronous lstatSync on the same tree, so the bound protects nothing there. What it adds is a way to lose the "repo-supplied hooks run on this machine" warning while the cap is engaged, and that warning is one of the clone feature's guarantees. The previous synchronous check (or an ENOENT-aware lstat like your pathExistsForWrite) is right here.

4. stripCaseEnvKeys is a writer but uses the read-side probe (src/hooks-config.ts:579). It runs inside withSafeSettingsWrite, which has already lstated the same file without a bound, so the bounded probe buys nothing. A false negative, though, leaves behind the stale entry this function exists to remove, and it keeps shadowing the fresh per-session override. Please use pathExistsForWrite() here, as your own docblock says writers should, or drop the check and let the existing readFile catch handle ENOENT.

5. Session creation still stats the linked path synchronously (src/web/routes/session-routes.ts:3661, existsSync(resolvedCasePath) in POST /api/quick-start, and :951, statSync(workingDir) in POST /api/sessions). The description says the case list and session creation were blocking. The list is fixed, but starting a session in the dead case through quick-start (the TUI, the codeman skill, any API client) or POST /api/sessions still blocks the event loop. Either bound these two as well (the same one-line swap, and POST /api/sessions already maps a non-directory to INVALID_INPUT), or reword the title and description so the changelog doesn't promise more than ships.

Small ones

6. On your barrel question: yes, please export boundedPathExists from src/utils/index.ts and import it from ./utils in case-routes.ts and hooks-config.ts. That's the import convention in CLAUDE.md.
7. PROBE_TIMEOUT_MS and MAX_STALLED_PROBES are operational limits, and limits live in src/config/, ideally env-overridable like the others. A slow but healthy sshfs that needs 2 s now drops out of the case list on first touch, so someone may well want to tune it. It's worth a sentence in the description too.

Once items 1 and 2 are in with their tests, this gets another review, and 3 and 4 are one-line swaps that can ride the same push. Thanks again for chasing this one down. The freeze is real, and the hard part of the fix is already done well.

Thanks for the careful review, and no worries at all about the DRAFT_COMMENT placeholder; this was well worth the wait. The throwaway tests on my own route harness made both must-fix items very concrete. I've rebased onto 1.34.0 and pushed one new commit on top of the original (d1bfbb4):

1. Absent vs unknown. The probe is now tri-state (present, absent, unknown), and only ENOENT/ENOTDIR count as absent.
   • GET /api/cases/:name keeps NOT_FOUND for a definite absence. An unreachable linked case answers with its registered path plus unreachable: true, and an unreachable local case answers OPERATION_FAILED.
   • runClaude and runShell now create a case only on errorCode === 'NOT_FOUND'.
   • The case list also keeps an unreachable linked case, marked unreachable, so it no longer looks deleted, and fix-plan reports unknown as an error rather than "no plan".
   • Tests cover the timeout path of the route, an EIO case, and both launchers.
2. Stall cap. I went with both of your options.
   • A stalled path now takes only its own mount out of probing (the deepest mount point from /proc/self/mounts, never /), and the global cap is a backstop that answers "unknown".
   • A single-path user request (opening a case, starting a session) can probe past the cap, still bounded.
   • Hooks install in any workspace that is not itself on the dead mount, and the statusLine exporter never overrides a settings file it could not read.
   • There is one console.warn when a path first stalls and one when the cap engages.
   • The /healthy/three test is replaced by one showing a healthy path probing true with two unrelated paths stalled. There are also caller-level tests for hooks, statusLine and GET with the cap engaged.
3. Clone warning. repoShipsClaudeSettings is back on its synchronous check, with a test that the warning survives the cap.
4. stripCaseEnvKeys uses pathExistsForWrite, with a test.
5. Session creation. I bounded both stats rather than rewording.
   • Missing and non-directory keep INVALID_INPUT. A folder that does not answer gets OPERATION_FAILED, and quick-start never scaffolds over it.
   • Route tests simulate the hard mount as a frozen synchronous call and a never-settling async stat.
   • One small behaviour change: an unreadable (EACCES) workingDir now reports "not responding or not readable" instead of "does not exist".
6. Barrel. The probe is exported from src/utils/index.ts and imported from ./utils.
7. Config. The limits moved to src/config/path-probe.ts and can be overridden with CODEMAN_PATH_PROBE_TIMEOUT_MS and CODEMAN_PATH_PROBE_MAX_STALLED. Both are in the Settings Reference and the description. I raised the cap default to 3, since with mount scoping it now only engages after three unrelated places stop answering. Happy to put it back to 2.

The title and description now cover session creation too. Every new test failed before its fix, and reverting each of the fixes for items 1, 2, 3 and 5 makes its tests fail. Typecheck, lint, format, the frontend-syntax and browser-exclude checks, and the test files touching these modules all pass.

Thanks for the quick and careful turnaround, @aakhter. This PR makes every probe of a linked case folder or session workingDir bounded and tri-state, so an unreachable network mount can no longer freeze the server. All seven items from the last round are in, each with a test, and typecheck, lint, format, the frontend checks and the full test suite pass here.

Three edge cases in the new mount-scoping and pastCap code need one more small round. I reproduced each one with a throwaway test.

1. Under the stall cap, applyWorkspaceHooks recreates a deleted workspace (src/hooks-config.ts:855). absentOrUnreachable() returns false for an unknown that is not near a stalled path. A probe refused by the cap is exactly that, even when the path is gone. ensureCodemanHooks then runs mkdir -p on .claude, which is the resurrection the old existsSync guard prevented (see the docblock at :840 and docs/architecture-invariants.md:322). With three unrelated paths stalled, applyWorkspaceHooks('<tmp>/deleted-repo', true) created <tmp>/deleted-repo/.claude/settings.local.json. When the probe answered unknown, please check existence with your pathExistsForWrite(workspace) before installing, and add a test that engages the cap and checks a deleted workspace stays deleted.

2. pastCap has no ceiling (src/utils/bounded-path-probe.ts:133). Each pastCap probe of a new path that hangs adds one stalled stat, so the count can reach libuv's default pool of 4. Then every async fs, dns.lookup and crypto call in the process waits on the dead mount. Your own "lets a pastCap probe through the cap" test ends with 4 hung stats. Mount scoping keeps this rare on Linux, but two common setups still get there:

• macOS has no /proc/self/mounts, so each path is its own scope, and four linked cases on one NAS are enough.
• On Linux, the same happens for linked paths reached through a symlink whose string prefix is on /.

Please give pastCap a hard ceiling that keeps one worker free (for example stalled.size < (Number(process.env.UV_THREADPOOL_SIZE) || 4) - 1, or put the bulk default back to 2 and let pastCap go to 3), with a test.

3. The stall scope cannot see through symlinks (src/utils/bounded-path-probe.ts:98). stallScope() takes the deepest mount whose path is a string prefix of the stalled path, and linked paths are stored as typed. Take /home as its own local mount and a linked case at ~/nas/project (~/nas -> /mnt/nas): a stall there records /home as the scope. Every path under it, ~/codeman-cases/* included, then reads unknown even with pastCap, so session creation, Run and hook installation fail across the home directory until the NAS comes back. /proc/self/mounts has the fs type in the third field. Please widen to the mount only for network and FUSE types (nfs, nfs4, cifs, smb3, smbfs, 9p, ceph, glusterfs, afs, lustre, davfs, fuse.*) and narrow to the stalled path otherwise, with a test using a local /home row.

Two small ones that can ride along:

• src/config/path-probe.ts:31 still says a stall takes out its "same parent directory". It scopes by mount point now.
• src/web/routes/case-routes.ts:1668: GET /api/cases/:name probes the folder with pastCap but its CLAUDE.md without it, so a healthy case reads hasClaudeMd: false under the cap. Using probePath(..., { pastCap: true }) there keeps the two consistent.

Once 1 to 3 are in with their tests, this is ready to merge. Thanks again: the freeze is real, and the probe itself (timers, in-flight sharing, no stray rejections) is in good shape.